-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix song select rewind logic not handling deleted beatmaps #23312
Conversation
Check if the poped beatmap exists within the beatmapSets
Please add test coverage for the bugged scenario in this pull request. See |
@@ -540,7 +540,7 @@ public void SelectPreviousRandom() | |||
{ | |||
var beatmap = randomSelectedBeatmaps.Pop(); | |||
|
|||
if (!beatmap.Filtered.Value) | |||
if (!beatmap.Filtered.Value && beatmapSets.Any(beatset => beatset.Beatmaps.Contains(beatmap))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably not be checking all beatmaps like that and be doing
if (!beatmap.Filtered.Value && beatmapSets.Any(beatset => beatset.Beatmaps.Contains(beatmap))) | |
if (!beatmap.Filtered.Value && beatmap.BeatmapSet?.PendingDelete != false) |
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only way to access the PendingDelete
flag would be by: beatmap.BeatmapInfo.BeatmapSet.PendingDelete
. And apparently, that doesn't look to get updated when the beatmap is actually deleted.
I've went with a different solution that's still magnitudes more efficient than iterating through all beatmaps in the carousel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably okay
Switch from stack to list wasn't handled correctly. Rewind now selects the last beatmap from list but removes the first instance from the list, so if there are multiple instance of the same beatmap, rewind will keep selecting the same beatmap until all instances are removed. Easy to reproduce if you search for something that returns 5 or so beatmaps, use random 15 times, rewind will select each beatmap 3 times before selecting the next in list. |
@LittleEndu can you open an actual issue please thanks |
When you rewind to a deleted beatmap, the game still selects the deleted beatmap, glitching the carousel, now it should check if the beatmap to rewind exists within
beatmapSets
Before: